Add WorkflowStep Factory and implement XContent-based Template Parsing #47

dbwiddis · 2023-09-19T17:49:27Z

Description

Creates a WorkflowStepFactory class to map a template "step type" to the correct instance
Parses the entire Use Case Template into XContent
Process the user inputs section for the workflow into WorkflowData params
Processes the user inputs section for each node into WorkflowData content
Refactors code to separate parsing functionality (WorkflowNode et. al.) from execution functionality (ProcessNode et. al.). Uses the WorfklowStep and Workflow Data when creating the ProcessNode.
Implements an executor service to use the OpenSearch thread pool for the async execution.
Added tests.
Fixed the ProcessNodeTests bug

Issues Resolved

Fixes #41
Fixes #42
Fixes #46
Fixes #50
Fixes #55

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

codecov · 2023-09-21T23:29:10Z

Codecov Report

Merging #47 (2fca71b) into main (1d22bee) will increase coverage by 13.05%.
The diff coverage is 90.14%.

@@              Coverage Diff              @@
##               main      #47       +/-   ##
=============================================
+ Coverage     73.87%   86.93%   +13.05%     
- Complexity       65      163       +98     
=============================================
  Files             8       13        +5     
  Lines           245      574      +329     
  Branches         22       75       +53     
=============================================
+ Hits            181      499      +318     
+ Misses           54       52        -2     
- Partials         10       23       +13

Files	Coverage Δ
...nsearch/flowframework/model/PipelineProcessor.java	`100.00% <100.00%> (ø)`
...a/org/opensearch/flowframework/model/Workflow.java	`100.00% <100.00%> (ø)`
...search/flowframework/workflow/CreateIndexStep.java	`84.84% <100.00%> (ø)`
...owframework/workflow/CreateIngestPipelineStep.java	`90.62% <ø> (ø)`
...opensearch/flowframework/workflow/ProcessNode.java	`58.69% <88.88%> (ø)`
.../opensearch/flowframework/FlowFrameworkPlugin.java	`0.00% <0.00%> (ø)`
...g/opensearch/flowframework/model/WorkflowEdge.java	`84.84% <84.84%> (ø)`
.../flowframework/workflow/WorkflowProcessSorter.java	`92.85% <92.85%> (ø)`
...g/opensearch/flowframework/model/WorkflowNode.java	`90.54% <90.54%> (ø)`
...a/org/opensearch/flowframework/model/Template.java	`92.36% <92.36%> (ø)`
... and 1 more

dbwiddis · 2023-09-25T21:14:00Z

Flipping back to draft to integrate the commits from #24 and #44 into the factory class (and delete my demo version).

Signed-off-by: Daniel Widdis <[email protected]>

dbwiddis · 2023-09-26T02:59:45Z

And flipping back to ready for review, after rebasing and also completing tasks from #42 and fixing #50.

joshpalis

initial pass, still need to go over the test classes

src/main/java/org/opensearch/flowframework/template/Template.java

src/main/java/org/opensearch/flowframework/workflow/Workflow.java

Signed-off-by: Daniel Widdis <[email protected]>

src/main/java/org/opensearch/flowframework/template/WorkflowNode.java

dbwiddis · 2023-09-26T18:43:47Z

To ease review, a general summary of the main part of this PR:

Parsing. Starts with Template, which also parses workflow, which also parses WorkflowEdge and WorkflowNode.
Process Sorting: takes the WorflowNodes and WorkflowEdges and creates the sorted ProcessNode sequence with links to predecessor ProcessNodes (no change from previous code, just separated out the parsing part)
WorkflowStepFactory: takes the node "type" field and generates a unique instance of the correct workflow step

Additional changes:

Made the sorter and step factory singletons taking the needed arguments that will go into the process nodes and workflow steps
Passed an executor service around to have better control of threading. This can be changed in the future away from the generic one.
Misc. cleanup of warnings (casting, deprecation)

Signed-off-by: Daniel Widdis <[email protected]>

owaiskazi19

Initial Pass! Thanks for adding XContent functionality to the project

build.gradle

src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java

src/main/java/org/opensearch/flowframework/template/ProcessNode.java

src/main/java/org/opensearch/flowframework/template/Template.java

src/main/java/org/opensearch/flowframework/template/WorkflowNode.java

src/main/java/org/opensearch/flowframework/workflow/Workflow.java

src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java

Signed-off-by: Daniel Widdis <[email protected]>

src/main/java/org/opensearch/flowframework/model/PipelineProcessor.java

owaiskazi19

Looks good overall with a minor comment/question.

joshpalis · 2023-09-27T17:56:22Z

Thanks @dbwiddis for adding the Pipeline Processor parsers!

#47) * Add WorkflowStepFactory class Signed-off-by: Daniel Widdis <[email protected]> * Add XContent classes representing Template JSON Signed-off-by: Daniel Widdis <[email protected]> * Add parse methods for the Template XContent Signed-off-by: Daniel Widdis <[email protected]> * Cleanup parsing, javadocs, and demo output Signed-off-by: Daniel Widdis <[email protected]> * Refactor to use field name constants, get tests working again Signed-off-by: Daniel Widdis <[email protected]> * Separate WorkflowNode and ProcessNode functionality Signed-off-by: Daniel Widdis <[email protected]> * Fix demos to align with template field names Signed-off-by: Daniel Widdis <[email protected]> * Add workflow, node, and edge tests Signed-off-by: Daniel Widdis <[email protected]> * Add Template tests Signed-off-by: Daniel Widdis <[email protected]> * Refactor TemplateParser to WorkflowProcessSorter Signed-off-by: Daniel Widdis <[email protected]> * Test exceptional cases Signed-off-by: Daniel Widdis <[email protected]> * Finish up exceptional cases Signed-off-by: Daniel Widdis <[email protected]> * Fix a template field name bug in demo Signed-off-by: Daniel Widdis <[email protected]> * Rebase with #34 Signed-off-by: Daniel Widdis <[email protected]> * Rebase changes from #54 Signed-off-by: Daniel Widdis <[email protected]> * Integrate thread pool executor service Signed-off-by: Daniel Widdis <[email protected]> * Fix flaky ProcessNodeTests by removing orTimeout Signed-off-by: Daniel Widdis <[email protected]> * Rebase and refactor with #44 Signed-off-by: Daniel Widdis <[email protected]> * Fix demos and remove DataDemo Signed-off-by: Daniel Widdis <[email protected]> * Use non-deprecated mapping method for CreateIndexStep Signed-off-by: Daniel Widdis <[email protected]> * Eliminate casting and deprecation warnings on test classes Signed-off-by: Daniel Widdis <[email protected]> * Remove unused/leftover demo class Signed-off-by: Daniel Widdis <[email protected]> * Typo Signed-off-by: Daniel Widdis <[email protected]> * Don't offer steps as an alternative to nodes Signed-off-by: Daniel Widdis <[email protected]> * Move Workflow into package with all the other parsing classes Signed-off-by: Daniel Widdis <[email protected]> * Move process sequencing classes into workflow package Signed-off-by: Daniel Widdis <[email protected]> * Add PipelineProcessor class and XContent parsing, rename package Signed-off-by: Daniel Widdis <[email protected]> --------- Signed-off-by: Daniel Widdis <[email protected]> (cherry picked from commit 734f9c2) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…Template Parsing (#60) Add WorkflowStep Factory and implement XContent-based Template Parsing (#47) * Add WorkflowStepFactory class * Add XContent classes representing Template JSON * Add parse methods for the Template XContent * Cleanup parsing, javadocs, and demo output * Refactor to use field name constants, get tests working again * Separate WorkflowNode and ProcessNode functionality * Fix demos to align with template field names * Add workflow, node, and edge tests * Add Template tests * Refactor TemplateParser to WorkflowProcessSorter * Test exceptional cases * Finish up exceptional cases * Fix a template field name bug in demo * Rebase with #34 * Rebase changes from #54 * Integrate thread pool executor service * Fix flaky ProcessNodeTests by removing orTimeout * Rebase and refactor with #44 * Fix demos and remove DataDemo * Use non-deprecated mapping method for CreateIndexStep * Eliminate casting and deprecation warnings on test classes * Remove unused/leftover demo class * Typo * Don't offer steps as an alternative to nodes * Move Workflow into package with all the other parsing classes * Move process sequencing classes into workflow package * Add PipelineProcessor class and XContent parsing, rename package --------- (cherry picked from commit 734f9c2) Signed-off-by: Daniel Widdis <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

github-actions bot added the backport 2.x backport PRs to 2.x branch label Sep 19, 2023

dbwiddis mentioned this pull request Sep 21, 2023

Adds Create Ingest Pipeline Step #44

Merged

owaiskazi19 mentioned this pull request Sep 21, 2023

Adds Create Index Building block #38

Merged

dbwiddis mentioned this pull request Sep 22, 2023

[DISCUSS] How should we parse the Template JSON? #46

Closed

dbwiddis changed the title ~~[WIP] WorkflowStep Factory~~ Add WorkflowStep Factory and implement XContent-based Template Parsing Sep 22, 2023

dbwiddis marked this pull request as ready for review September 23, 2023 06:01

dbwiddis requested review from owaiskazi19, joshpalis, ohltyler, amitgalitz and jackiehanyang as code owners September 23, 2023 06:01

This was referenced Sep 25, 2023

[FEATURE] Simplify WorkflowData to a concrete class #53

Closed

Change WorkflowData from interface to class #54

Merged

[FEATURE] Create an XContent parser for Processors in pipelines. #55

Closed

dbwiddis marked this pull request as draft September 25, 2023 21:14

dbwiddis added 13 commits September 25, 2023 18:15

Add WorkflowStepFactory class

24b3542

Signed-off-by: Daniel Widdis <[email protected]>

Add XContent classes representing Template JSON

e70b9e8

Signed-off-by: Daniel Widdis <[email protected]>

Add parse methods for the Template XContent

677cc24

Signed-off-by: Daniel Widdis <[email protected]>

Cleanup parsing, javadocs, and demo output

57fa740

Signed-off-by: Daniel Widdis <[email protected]>

Refactor to use field name constants, get tests working again

1214ebc

Signed-off-by: Daniel Widdis <[email protected]>

Separate WorkflowNode and ProcessNode functionality

fd72b2d

Signed-off-by: Daniel Widdis <[email protected]>

Fix demos to align with template field names

f2441b3

Signed-off-by: Daniel Widdis <[email protected]>

Add workflow, node, and edge tests

c7a2371

Signed-off-by: Daniel Widdis <[email protected]>

Add Template tests

388741c

Signed-off-by: Daniel Widdis <[email protected]>

Refactor TemplateParser to WorkflowProcessSorter

df4a0d8

Signed-off-by: Daniel Widdis <[email protected]>

Test exceptional cases

8589cfa

Signed-off-by: Daniel Widdis <[email protected]>

Finish up exceptional cases

309d059

Signed-off-by: Daniel Widdis <[email protected]>

Fix a template field name bug in demo

2804a52

Signed-off-by: Daniel Widdis <[email protected]>

dbwiddis added 6 commits September 25, 2023 18:16

Integrate thread pool executor service

90e95e3

Signed-off-by: Daniel Widdis <[email protected]>

Fix flaky ProcessNodeTests by removing orTimeout

3fa4d0a

Signed-off-by: Daniel Widdis <[email protected]>

Rebase and refactor with #44

62fa53c

Signed-off-by: Daniel Widdis <[email protected]>

Fix demos and remove DataDemo

6c86dd1

Signed-off-by: Daniel Widdis <[email protected]>

Use non-deprecated mapping method for CreateIndexStep

c11a5bf

Signed-off-by: Daniel Widdis <[email protected]>

Eliminate casting and deprecation warnings on test classes

adc349a

Signed-off-by: Daniel Widdis <[email protected]>

dbwiddis marked this pull request as ready for review September 26, 2023 02:32

Remove unused/leftover demo class

21b530b

Signed-off-by: Daniel Widdis <[email protected]>

joshpalis reviewed Sep 26, 2023

View reviewed changes

dbwiddis added 2 commits September 26, 2023 10:33

Typo

8d28308

Signed-off-by: Daniel Widdis <[email protected]>

Don't offer steps as an alternative to nodes

555d88b

Signed-off-by: Daniel Widdis <[email protected]>

joshpalis reviewed Sep 26, 2023

View reviewed changes

src/main/java/org/opensearch/flowframework/template/WorkflowNode.java Outdated Show resolved Hide resolved

dbwiddis added 2 commits September 26, 2023 12:33

Move Workflow into package with all the other parsing classes

5c78046

Signed-off-by: Daniel Widdis <[email protected]>

Move process sequencing classes into workflow package

87685ff

Signed-off-by: Daniel Widdis <[email protected]>

owaiskazi19 reviewed Sep 26, 2023

View reviewed changes

dbwiddis requested review from owaiskazi19 and joshpalis September 27, 2023 05:11

Add PipelineProcessor class and XContent parsing, rename package

2fca71b

Signed-off-by: Daniel Widdis <[email protected]>

owaiskazi19 reviewed Sep 27, 2023

View reviewed changes

src/main/java/org/opensearch/flowframework/model/PipelineProcessor.java Show resolved Hide resolved

owaiskazi19 approved these changes Sep 27, 2023

View reviewed changes

joshpalis approved these changes Sep 27, 2023

View reviewed changes

dbwiddis merged commit 734f9c2 into opensearch-project:main Sep 28, 2023
10 checks passed

dbwiddis deleted the step-factory branch September 28, 2023 00:58

opensearch-trigger-bot bot mentioned this pull request Sep 28, 2023

[Backport 2.x] Add WorkflowStep Factory and implement XContent-based Template Parsing #60

Merged

dbwiddis mentioned this pull request Sep 28, 2023

[FEATURE] Better Thread Pool integration #61

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add WorkflowStep Factory and implement XContent-based Template Parsing #47

Add WorkflowStep Factory and implement XContent-based Template Parsing #47

dbwiddis commented Sep 19, 2023 •

edited

Loading

codecov bot commented Sep 21, 2023 •

edited

Loading

dbwiddis commented Sep 25, 2023

dbwiddis commented Sep 26, 2023

joshpalis left a comment

dbwiddis commented Sep 26, 2023

owaiskazi19 left a comment

owaiskazi19 left a comment

joshpalis commented Sep 27, 2023

Add WorkflowStep Factory and implement XContent-based Template Parsing #47

Add WorkflowStep Factory and implement XContent-based Template Parsing #47

Conversation

dbwiddis commented Sep 19, 2023 • edited Loading

Description

Issues Resolved

codecov bot commented Sep 21, 2023 • edited Loading

Codecov Report

dbwiddis commented Sep 25, 2023

dbwiddis commented Sep 26, 2023

joshpalis left a comment

Choose a reason for hiding this comment

dbwiddis commented Sep 26, 2023

owaiskazi19 left a comment

Choose a reason for hiding this comment

owaiskazi19 left a comment

Choose a reason for hiding this comment

joshpalis commented Sep 27, 2023

dbwiddis commented Sep 19, 2023 •

edited

Loading

codecov bot commented Sep 21, 2023 •

edited

Loading